Use aiohasupervisor for addon info calls #125926

mdegat01 · 2024-09-13T20:04:38Z

Proposed change

Begin the process of replacing hassio/handler with a published client library on pip in aiohasupervisor. As this is our guidance to all other integration developers, our supervisor integration should follow this as well.

Since the hassio component's internal library is used all over integrations for access to supervisor information replacing handler will likely take a good number of PRs. This PR seeks only to replace the method used for obtaining addon info with the equivalent method from the library.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

mdegat01 · 2024-09-13T21:14:05Z

Not sure how the errors in the hassio/issues tests didn't show up when I was running them locally but I'll fix those. The bluetooth ones I'm seeing the same failures when I switch to dev branch locally so I don't think those are a result of this PR? Will circle back.

mdegat01 · 2024-09-16T21:12:13Z

I'm going to call this ready for review. Yes there is a failure in a test in the Plugwise integration but the test appears to be flaky. I ran it locally against dev branch and it failed 2 times in 10 with no changes in between. I opened #126086 for it.

homeassistant/components/hassio/addon_manager.py

joostlek · 2024-09-17T14:04:17Z

homeassistant/components/hassio/handler.py

+        self._client = SupervisorClient(
+            base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession
+        )
+
+    @property
+    def client(self) -> SupervisorClient:
+        """Return aiohasupervisor client."""
+        return self._client


Suggested change

self._client = SupervisorClient(

base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession

)

@property

def client(self) -> SupervisorClient:

"""Return aiohasupervisor client."""

return self._client

self.client = SupervisorClient(

base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession

)

Why make it protected if you're going to expose it anyway :)

Because if you do it that way then someone can do this:

hassio = HassIO(...) hassio.client = SupervisorClient(...)

By making it a property like that client is read-only on instances of HassIO. Others can access it to make calls to supervisor, they cannot change its value to something new.

Well They can but they'd have to set hassio._client in my example above and then linting would tell them not to do that.

I am not aware if we have places in the code where we explictly do this, let me check with others

Okay, talked with Martin and this would be good in its current form

homeassistant/components/hassio/handler.py

joostlek · 2024-09-17T14:05:21Z

homeassistant/components/hassio/handler.py

+def get_supervisor_client(hass: HomeAssistant) -> SupervisorClient:
+    """Return supervisor client."""
+    hassio: HassIO = hass.data[DOMAIN]
+    return hassio.client


You can use HassKey in a future PR to add typing to the hass.data so you can avoid this type overwrite (if wanted)

Wasn't aware of that, that would be nice. It would definitely have to be a follow-up PR though as doing that would require a significant refactor to this integration looking at how much we use that currently.

tests/components/hassio/test_binary_sensor.py

tests/components/hassio/test_init.py

home-assistant · 2024-09-17T14:11:32Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

MartinHjelmare · 2024-09-19T08:21:50Z

homeassistant/components/otbr/config_flow.py

@@ -43,6 +39,12 @@ class AlreadyConfigured(HomeAssistantError):
    """Raised when the router is already configured."""


+@callback
+def get_addon_manager(hass: HomeAssistant, slug: str) -> AddonManager:


Each add-on manager should be a singleton per add-on. We can use the singleton decorator. See other integrations that create add-on managers.

sdb9696 · 2024-09-19T12:51:47Z

Hi, I just did a fresh installation of HA core from dev using script/setup and got a missing import error for aiohasupervisor. I think this PR is the cause although I'm not clear why a core install loads hassio as I thought that was the supervisor.

To reproduce

MartinHjelmare · 2024-09-19T12:53:05Z

We're aware and a solution is under way.

MartinHjelmare · 2024-09-19T13:37:21Z

The missing import problem should be solved by:
#126225

mdegat01 requested review from a team and ludeeus as code owners September 13, 2024 20:04

home-assistant bot added cla-signed code-quality core has-tests labels Sep 13, 2024

mdegat01 force-pushed the use-aiohasupervisor-library branch from 5c16f0c to ef4b501 Compare September 13, 2024 20:06

mdegat01 marked this pull request as draft September 13, 2024 21:13

mdegat01 force-pushed the use-aiohasupervisor-library branch from ef4b501 to ff6a53b Compare September 16, 2024 20:03

mdegat01 mentioned this pull request Sep 16, 2024

Flaky test in Plugwise integration #126086

Closed

mdegat01 marked this pull request as ready for review September 16, 2024 21:11

mdegat01 force-pushed the use-aiohasupervisor-library branch from ff6a53b to 87f04ee Compare September 16, 2024 21:12

joostlek requested changes Sep 17, 2024

View reviewed changes

home-assistant bot marked this pull request as draft September 17, 2024 14:11

mdegat01 force-pushed the use-aiohasupervisor-library branch from 87f04ee to 31b51bf Compare September 17, 2024 16:42

mdegat01 added 3 commits September 17, 2024 18:01

Use aiohasupervisor for addon info calls

1952d42

Fix issue/repair tests in supervisor

4d21624

Fixes from feedback

8312ef4

mdegat01 force-pushed the use-aiohasupervisor-library branch from 31b51bf to 8312ef4 Compare September 17, 2024 18:02

mdegat01 marked this pull request as ready for review September 17, 2024 18:29

home-assistant bot requested a review from joostlek September 17, 2024 18:29

joostlek approved these changes Sep 17, 2024

View reviewed changes

joostlek merged commit 97d0d91 into dev Sep 17, 2024
39 of 40 checks passed

joostlek deleted the use-aiohasupervisor-library branch September 17, 2024 21:22

github-actions bot locked and limited conversation to collaborators Sep 18, 2024

farmio mentioned this pull request Sep 19, 2024

docker container is not starting with commit 97d0d91d in dev-branch #126250

Closed

MartinHjelmare reviewed Sep 19, 2024

View reviewed changes

home-assistant unlocked this conversation Sep 19, 2024

github-actions bot locked and limited conversation to collaborators Sep 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use aiohasupervisor for addon info calls #125926

Use aiohasupervisor for addon info calls #125926

mdegat01 commented Sep 13, 2024

mdegat01 commented Sep 13, 2024

mdegat01 commented Sep 16, 2024

joostlek Sep 17, 2024

mdegat01 Sep 17, 2024 •

edited

Loading

joostlek Sep 17, 2024

joostlek Sep 17, 2024

joostlek Sep 17, 2024

mdegat01 Sep 17, 2024

home-assistant bot commented Sep 17, 2024

MartinHjelmare Sep 19, 2024

sdb9696 commented Sep 19, 2024

MartinHjelmare commented Sep 19, 2024

MartinHjelmare commented Sep 19, 2024

Use aiohasupervisor for addon info calls #125926

Use aiohasupervisor for addon info calls #125926

Conversation

mdegat01 commented Sep 13, 2024

Proposed change

Type of change

Additional information

Checklist

mdegat01 commented Sep 13, 2024

mdegat01 commented Sep 16, 2024

joostlek Sep 17, 2024

Choose a reason for hiding this comment

mdegat01 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

joostlek Sep 17, 2024

Choose a reason for hiding this comment

joostlek Sep 17, 2024

Choose a reason for hiding this comment

joostlek Sep 17, 2024

Choose a reason for hiding this comment

mdegat01 Sep 17, 2024

Choose a reason for hiding this comment

home-assistant bot commented Sep 17, 2024

MartinHjelmare Sep 19, 2024

Choose a reason for hiding this comment

sdb9696 commented Sep 19, 2024

MartinHjelmare commented Sep 19, 2024

MartinHjelmare commented Sep 19, 2024

mdegat01 Sep 17, 2024 •

edited

Loading